Closed
Bug 1376158
Opened 8 years ago
Closed 8 years ago
Stack exhaustion in [@ AppendText]
Categories
(Core :: MathML, defect)
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox54 | --- | unaffected |
firefox55 | --- | fixed |
firefox56 | --- | fixed |
People
(Reporter: tsmith, Assigned: emilio)
References
(Blocks 1 open bug)
Details
(Keywords: crash, testcase)
Attachments
(2 files)
75 bytes,
text/html
|
Details | |
59 bytes,
text/x-review-board-request
|
xidorn
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
#0 0x00007fffdcaf92d5 in AppendText () at src/dom/base/nsLineBreaker.cpp:330
#1 0x00007fffe0d4737b in SetupBreakSinksForTextRun () at src/layout/generic/nsTextFrame.cpp:2647
#2 0x00007fffe0d40874 in BuildTextRunForFrames () at src/layout/generic/nsTextFrame.cpp:2408
#3 0x00007fffe0d3a8f7 in FlushFrames () at src/layout/generic/nsTextFrame.cpp:1688
#4 0x00007fffe0d4ae4f in BuildTextRuns () at src/layout/generic/nsTextFrame.cpp:1614
#5 EnsureTextRun () at src/layout/generic/nsTextFrame.cpp:2861
#6 0x00007fffe0d82736 in AddInlineMinISizeForFlow () at src/layout/generic/nsTextFrame.cpp:8455
#7 0x00007fffe0d85464 in AddInlineMinISize () at src/layout/generic/nsTextFrame.cpp:8625
#8 0x00007fffe0b090cf in GetMinISize () at src/layout/generic/nsBlockFrame.cpp:770
#9 0x00007fffe0b7b356 in ShrinkWidthToFit () at src/layout/generic/nsFrame.cpp:5800
#10 ComputeAutoSize () at src/layout/generic/nsContainerFrame.cpp:845
#11 0x00007fffe0b81f97 in ComputeSize () at src/layout/generic/nsFrame.cpp:5061
#12 0x00007fffe0ac3b61 in InitConstraints () at src/layout/generic/ReflowInput.cpp:2492
#13 0x00007fffe0aba110 in Init () at src/layout/generic/ReflowInput.cpp:409
#14 0x00007fffe10e7cce in Reflow () at src/layout/mathml/nsMathMLTokenFrame.cpp:143
#15 0x00007fffe0af58a4 in ReflowChild () at src/layout/generic/nsContainerFrame.cpp:978
#16 0x00007fffe10d8d96 in ReflowChild () at src/layout/mathml/nsMathMLContainerFrame.cpp:839
#17 0x00007fffe10d96cd in Reflow () at src/layout/mathml/nsMathMLContainerFrame.cpp:892
#18 0x00007fffe0af58a4 in ReflowChild () at src/layout/generic/nsContainerFrame.cpp:978
#19 0x00007fffe10d8d96 in ReflowChild () at src/layout/mathml/nsMathMLContainerFrame.cpp:839
#20 0x00007fffe10d96cd in Reflow () at src/layout/mathml/nsMathMLContainerFrame.cpp:892
#21 0x00007fffe0cc71df in ReflowFrame () at src/layout/generic/nsLineLayout.cpp:921
#22 0x00007fffe0cc5153 in ReflowInlineFrame () at src/layout/generic/nsInlineFrame.cpp:798
#23 0x00007fffe0cc3597 in ReflowFrames () at src/layout/generic/nsInlineFrame.cpp:681
#24 0x00007fffe0cc26ba in Reflow () at src/layout/generic/nsInlineFrame.cpp:460
Flags: in-testsuite?
Comment 1•8 years ago
|
||
INFO: Last good revision: aab0dfdae32f14246e3bed8824a5f7ce309276cd
INFO: First bad revision: 3e6477d932037d6026ac13bd8c988dc0a51935d4
INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=aab0dfdae32f14246e3bed8824a5f7ce309276cd&tochange=3e6477d932037d6026ac13bd8c988dc0a51935d4
Blocks: 1361766
status-firefox54:
--- → unaffected
status-firefox55:
--- → affected
status-firefox56:
--- → affected
status-firefox-esr52:
--- → unaffected
Flags: needinfo?(emilio+bugs)
Version: Trunk → 55 Branch
Assignee | ||
Comment 2•8 years ago
|
||
I don't know a lot about MathML, but I don't think that should be valid markup... Pretty much every change to the test case makes it invalid markup...
Here's a test-case with the balanced tags:
<math>
<munderover>
>
<munder accentunder="true">
<mo>)</mo>
<mscarry>
</munder>
</munderover>
</math>
I _think_ (reading https://www.w3.org/TR/MathML3/chapter3.html#presm.mscarries) that <mscarry> should only be allowed inside <mscarries>, but I'm not totally sure...
Loading this test-case in a debug build I get a bunch of:
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004001: file /home/emilio/projects/moz/stylo/layout/generic/nsBlockFrame.cpp, line 946...
Assignee | ||
Comment 3•8 years ago
|
||
That being said I think I know why the test-case regressed with my patch, which is kind of sad... MathML reflows are non-idempotent, which is something someone should really fix :(
Assignee | ||
Comment 4•8 years ago
|
||
Flags: needinfo?(emilio+bugs)
Comment hidden (mozreview-request) |
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8881163 [details]
Bug 1376158: Don't let non-primary frames affect the mIncrementScriptLevel of the content.
https://reviewboard.mozilla.org/r/152432/#review158550
Attachment #8881163 -
Flags: review?(xidorn+moz) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5280f9ba46df
Don't let non-primary frames affect the mIncrementScriptLevel of the content. r=xidorn
Comment 8•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment 9•8 years ago
|
||
Should we uplift this change to beta?
Assignee: nobody → emilio+bugs
Flags: needinfo?(emilio+bugs)
Assignee | ||
Comment 10•8 years ago
|
||
(In reply to Julien Cristau [:jcristau] from comment #9)
> Should we uplift this change to beta?
Seems harmless, yeah, will file a request.
Flags: needinfo?(emilio+bugs)
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8881163 [details]
Bug 1376158: Don't let non-primary frames affect the mIncrementScriptLevel of the content.
Approval Request Comment
[Feature/Bug causing the regression]: bug 1361766
[User impact if declined]: Crashes on some obscure MathML content.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: Just this patch.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: This just enforces an invariant that is assumed in the rest of the MathML code, and only affects already-invalid MathML content.
[String changes made/needed]: none
Attachment #8881163 -
Flags: approval-mozilla-beta?
Comment 12•8 years ago
|
||
Comment on attachment 8881163 [details]
Bug 1376158: Don't let non-primary frames affect the mIncrementScriptLevel of the content.
avoid stack exhaustion, regression in beta55
Attachment #8881163 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 13•8 years ago
|
||
bugherder uplift |
Comment 14•8 years ago
|
||
(In reply to Emilio Cobos Álvarez [:emilio] from comment #11)
> [Is this code covered by automated tests?]: Yes
> [Has the fix been verified in Nightly?]: Yes
> [Needs manual test from QE? If yes, steps to reproduce]: No
Setting qe-verify- based on Emilio's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•